Skip to content

waypoint: metadata discovery service client#4255

Merged
istio-testing merged 3 commits intoistio:masterfrom
kyessenov:wds
Feb 28, 2023
Merged

waypoint: metadata discovery service client#4255
istio-testing merged 3 commits intoistio:masterfrom
kyessenov:wds

Conversation

@kyessenov
Copy link
Contributor

@kyessenov kyessenov commented Nov 30, 2022 •
edited
Loading

Signed-off-by: Kuat Yessenov kuat@google.com

This PR adds a workload discovery client support to Envoy, which allows Envoy to retrieve additional metadata by an IP address using delta ADS .

Sample config:

dynamic_resources:
  ads_config:
    api_type: DELTA_GRPC
    transport_api_version: V3
    grpc_services:
      envoy_grpc:
        cluster_name: xds_cluster
bootstrap_extensions:
- name: metadata_discovery
  typed_config:
    "@type": type.googleapis.com/udpa.type.v1.TypedStruct
    type_url: type.googleapis.com/istio.workload.BootstrapExtension
    value:
      config_source:
        ads: {}

AGGREGATED_DELTA_GRPC needs to be completed in Envoy to support both delta and sotw ADS simulatenously.

Context: istio/istio#42070

@kyessenov kyessenov requested a review from a team November 30, 2022 18:15
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 30, 2022
@kyessenov kyessenov added the do-not-merge Block automatic merging of a PR. label Nov 30, 2022
@zirain
Copy link
Member

zirain commented Dec 1, 2022

can your share doc/PR about AGGREGATED_DELTA_GRPC ?

@kyessenov
Copy link
Contributor Author

kyessenov commented Dec 1, 2022

@zirain It's a hidden option in Envoy that's not fully implemented for muxing delta and sotw over ADS. I think it's probably easier to separate the delta stream from ADS initially, but need a PR in envoy to allow new delta services on top of builtin xDS.

@istio-testing istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 23, 2023
@istio-policy-bot istio-policy-bot added the lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. label Feb 24, 2023
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 24, 2023
@kyessenov kyessenov reopened this Feb 24, 2023
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 27, 2023 •
edited
Loading

CLA Not Signed

@istio-testing istio-testing added needs-rebase Indicates a PR needs to be rebased before being merged size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 27, 2023
@kyessenov
wip
3187630 
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov
complete
bbee848 
Signed-off-by: Kuat Yessenov <kuat@google.com>
@istio-testing istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Feb 27, 2023
@kyessenov
lint
c7c3624 
Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov
Copy link
Contributor Author

kyessenov commented Feb 27, 2023 •
edited
Loading

CC @costinm @howardjohn

This implements a waypoint telemetry discovery service using the shared WDS from istiod. This is strictly guarded to:

  • waypoint, backend IPs only;
  • optional, enabled via bootstrap extension;
  • confirmed to work with delta ADS, does not work with the per service delta xDS (upstream envoy issue);

With this change, we have uniform handling of "destination" metadata in the waypoints, that no longer depends on EDS metadata or some other ECDS tables. This allows us to drop the requirement to supply the baggage response header from the destination, improves the telemetry integrity, and supports the "direct" (VIP-less) data path metadata.

Some future work may include:

  • add on-demand support;
  • change workload key away from IP into something more trustworthy;
  • support for source telemetry to eliminate "baggage" request;
  • inclusion into sidecar xDS;

@kyessenov
Copy link
Contributor Author

kyessenov commented Feb 27, 2023

/retest

@kyessenov kyessenov changed the title metadata discovery service client waypoint: metadata discovery service client Feb 27, 2023
@kyessenov kyessenov removed the do-not-merge Block automatic merging of a PR. label Feb 27, 2023
@kyessenov
Copy link
Contributor Author

kyessenov commented Feb 27, 2023

/retest

kyessenov
kyessenov commented Feb 27, 2023
View reviewed changes
source/extensions/filters/http/istio_stats/istio_stats.cc
@@ -975,7 +1001,10 @@ class IstioStatsFilter : public Http::PassThroughFilter,
: context_.unknown_});
switch (config_->reporter()) {
case Reporter::ServerGateway: {
Copy link
Contributor Author

@kyessenov kyessenov Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main guard: only Waypoints set ServerGateway.

lei-tang
lei-tang approved these changes Feb 28, 2023
View reviewed changes
Copy link
Contributor

@lei-tang lei-tang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The PR is guarded for Ambient waypoints.

@kyessenov
Copy link
Contributor Author

kyessenov commented Feb 28, 2023

/retest

1 similar comment
@kyessenov
Copy link
Contributor Author

kyessenov commented Feb 28, 2023

/retest

@istio-testing istio-testing merged commit 9e04596 into istio:master Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lei-tang lei-tang lei-tang approved these changes

Assignees

No one assigned

Labels

lifecycle/automatically-closed Indicates a PR or issue that has been closed automatically. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kyessenov @zirain @lei-tang @istio-testing @istio-policy-bot